-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add attribute to fix B/C issue in list form field #44495
base: 5.2-dev
Are you sure you want to change the base?
Conversation
can you please remove all unrelated changes (code style) |
Co-authored-by: Harald Leithner <[email protected]>
Co-authored-by: Harald Leithner <[email protected]>
Co-authored-by: Harald Leithner <[email protected]>
Co-authored-by: Harald Leithner <[email protected]>
I see :( that means redoing the change outside my dev environment in a text editor. |
not sure why your dev environment restructure code automatically but ok, anyway you can use github editor todo this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it for you
you seem to use a mix of showonlocal and showonLocal |
Hopefully I found all now |
I am not in favor for this. |
you found all 4 of them :) |
and in this is exactly the issue: when a misconception is used for years, it is not a misconception anymore imo. developers depend on the functionality offered. |
This is why, disabled formatting in the editor so should be okay now: PHP code intelligence for Visual Studio Code.
|
I think it just continues to encourage developers to fall into the trap of creating dependencies that don't exist and continues to force misconceptions. But I think we can agree to disagree on that and that's fine too 🙂 |
@LadySolveig this PR forces nobody, it adds the possibility (for which there are use cases) to use the local value instead of the global value. Nobody is forced, the developer has to take action if he wants to use the local value. So overall an improvement imo |
I am not sure what you mean with 'chip in'. |
chip in is english slang To chip in can mean to interrupt with a comment or remark, similar to chime in Example: We can fix up this old house if we all chip in and work together |
I don't think that disabling the function via flag is the correct way forward, if this behaviour is needed (imho it's the wrong approach, but who is here to judge...), then we should improve the showOn JS adding more flexibility. |
hi @bembelimen , the 'flag' is added here:
and because that flag handled in showon.js in here: https://github.com/LadySolveig/joomla-cms/blob/52c6d325ae130ad6c54a6c585688d0c689bdcce9/build/media_source/system/js/showon.es6.js#L152-L157 This PR (other then reverting #43842 is the easiest fix. refactoring showon is not something I would take on as a fix: that would be something for a major upgrade. And I mean fix as #43842 adds a feature but by doing so it breaks the existing functionality of using the local value (as it is being used by all other form fields). |
Pull Request for Issue #44484 .
Summary of Changes
with the merging of #43842 the functionality that was present in Joomla for years has changed for extensions using that functionality.
because there is no way to restore the used and required functionality this can be considered a B/C.
Furthermore the PR introduced ambiguity as it changed the logic / functionality only for one field.
See discussion here: #44484
This PR adds an attribute with which a developer can 'restore' the showon functionality as it was, at least giving them the option to keep using the core ListField instead of developing a workaround.
this PR adds an form attribute: showonlocal="true/false" (default = false)
when setting showonlocal, the showon will acto on the local set value (like it has been for years in pré 5.2), when not set or when set to false, showon will act on the global value (like it is currently in 5.2)
pinging @LadySolveig
Testing Instructions
Let's first see if this can get some tracktion as setting up testing instructions take a lot of time. Time easily wasted :)
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed